Surface OAuth failures on server selection & support subpath-hosted HA - #24
Merged
Conversation
Two onboarding fixes from the code audit: - ServerSelectionView now renders viewModel.errorMessage via an alert, so a failed OAuth prepare (server card) or a failed token exchange in handleOAuthCallback is visible and dismissible instead of silently returning the user to the server list. The alert is gated to only present when no sheet is up; the manual-entry sheet keeps its inline validation text and clears the error on disappear so a manual-validation message never leaks into the alert. errorMessage is cleared when opening manual entry (prepareManualEntry). - OAuthFlowManager.prepare appends the callback/authorize paths onto the server URL's existing base path instead of overwriting it, so subpath-hosted HA (e.g. https://example.com/ha) builds correct same-origin URLs. Force-unwraps on the runtime URLComponents/URL are replaced with guard-let throwing AuthError.invalidServerURL. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Per project comment-style convention: multi-line comments use the /** ... */ block form. Converts the two-line base-path comment introduced in the previous commit; single-line // comments left as-is. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
connectAndDismiss dismissed the sheet whenever connectManual returned true, even when the subsequent connect()/startOAuth set an error synchronously (e.g. a URL with embedded credentials passes field validation but is rejected by OAuth preparation). The sheet's onDisappear then cleared errorMessage before the parent alert could present, so the attempt failed silently with no error shown anywhere. Now connectAndDismiss only dismisses when the attempt produced no error, keeping the inline error visible for all synchronous manual-entry failures — consistent with existing field-validation errors. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
viewModel.errorMessageonServerSelectionViewvia an.alert, so a failed OAuth prepare (from a discovered/manually-entered server) or a failed token exchange inhandleOAuthCallbackis now visible and dismissible instead of silently returning the user to the server list. The alert is gated to present only when no sheet is up.ManualEntrySheetkeeps its inline validation text and clearserrorMessageon disappear so a manual-validation message never leaks into the parent alert.connectAndDismissonly dismisses when the attempt produced no error, so all synchronous manual-entry failures (including a URL that passes field validation but is rejected by OAuth prep) stay visible inline rather than dismissing over an unshown error.errorMessageis cleared when opening manual entry (prepareManualEntry).OAuthFlowManager.preparenow appends the callback/authorize paths onto the server URL's existing base path instead of overwriting it, so subpath-hosted HA behind a reverse proxy (e.g.https://example.com/ha) builds correct same-origin URLs. Force-unwraps on the runtimeURLComponents/URLare replaced withguard letthrowingAuthError.invalidServerURL.Why
Two code-audit findings:
errorMessagewas set on two failure paths but only ever rendered inside the manual-entry sheet, so a failed connect looked like it silently did nothing.https://example.com/auth/authorizeinstead ofhttps://example.com/ha/auth/authorize).Notes
OAuthFlowManagerTests.connectAndDismissrelies on.HemeraTests(all green).